System tray #22
System tray #22
Conversation
There was a problem hiding this comment.
Thanks a bunch for the contribution! It's looking great, the code you wrote is really clean ^^
I have some comments on it below, they're mostly just nits and one missing part for the app config migration. Would you be able to update those last few things? Once those changes are in I'm pretty sure we can get this merged.
| oscReceivingPort: number; | ||
| enableDesktopNotifications: boolean; | ||
| enableXSOverlayNotifications: boolean; | ||
| exitInSystemTray: boolean; | ||
| startInSystemTray: boolean; | ||
| } |
There was a problem hiding this comment.
So these interfaces have a version property that gets bumped whenever they are modified. This is so that people's existing configurations can be migrated once they update.
In this case, two new fields got added, which means the version property should be bumped from 4 to 5, and a migration should be written in src/app/migrations/app-settings.migrations.ts that bumps this version and adds the missing fields. If you take a look at the previous migrations in there, you'll see it's quite simple!
| @@ -0,0 +1,48 @@ | |||
| import { Injectable } from '@angular/core'; | |||
| import { AppSettingsService } from './app-settings.service'; | |||
| import { debounceTime, filter, first, last, map, pairwise, take } from 'rxjs'; | |||
There was a problem hiding this comment.
Nit: there's still a few unused imports here :)
| map(settings => settings.exitInSystemTray), | ||
| pairwise(), | ||
| filter(([oldVal, newVal]) => oldVal !== newVal), | ||
| map(([_, newVal]) => newVal), |
There was a problem hiding this comment.
| map(([_, newVal]) => newVal), | |
| map(([, newVal]) => newVal), |
Nit: in the case of unused array parameters, they can be left empty!
| map(settings => settings.startInSystemTray), | ||
| pairwise(), | ||
| filter(([oldVal, newVal]) => oldVal !== newVal), | ||
| map(([_, newVal]) => newVal), |
There was a problem hiding this comment.
| map(([_, newVal]) => newVal), | |
| map(([, newVal]) => newVal), |
| <h2 translate>settings.general.systemTray.title</h2> | ||
| <div class="settings"> | ||
| <div class="setting-row"> | ||
| <div class="setting-row-label" translate> |
There was a problem hiding this comment.
| <div class="setting-row-label" translate> | |
| <div class="setting-row-label"> |
Nit: unneeded translate directive
| "title": "System tray", | ||
| "close": { | ||
| "label": "Exit in system tray", | ||
| "description": "Collapses the program into the system tray upon clicking on exit button" |
There was a problem hiding this comment.
| "description": "Collapses the program into the system tray upon clicking on exit button" | |
| "description": "When closing Oyasumi, it will continue to run in the system tray" |
| }, | ||
| "start": { | ||
| "label": "Start in system tray", | ||
| "description": "Starts the program reduced in the system tray." |
There was a problem hiding this comment.
| "description": "Starts the program reduced in the system tray." | |
| "description": "The main window will remain hidden when Oyasumi is started" |
| "close": { | ||
| "label": "Fermer l'application dans la barre d'état", | ||
| "description": "Réduit l'application dans la barre d'état système lorsque vous cliquez sur le bouton 'Fermer'" | ||
| }, | ||
| "start": { | ||
| "label": "Démarrer dans la barre d'état", | ||
| "description": "Démarrer l'application cachée dans la barre d'état. Vous pouvez la restaurer en cliquant sur l'icône dans la barre d'état." | ||
| } |
There was a problem hiding this comment.
Maybe these need to be updated based on the previous changes to the English translations?
| // Initializes the system tray with menus. | ||
| pub fn init_system_tray() -> SystemTray { | ||
| // Menus | ||
| let menu_quit = CustomMenuItem::new(QUIT, "Quit"); |
There was a problem hiding this comment.
We should probably find a way of localizing this as well, but I understand there's currently not yet a nice way of getting the translations to the rust side. I think doing that might be a bit outside of this PR's scope, especially when you consider the translations should change when the user switches language on the front, so that's probably something to look into after getting this merged.
tl;dr: this is fine as is, feel free to ignore. I've opened a separate issue for this in #23.
| // pub fn handle_window_exit<R: Runtime>(&self) -> impl Fn(GlobalWindowEvent<R>) + Send + Sync + 'static { | ||
| // return |event| match event.event() { | ||
| // tauri::WindowEvent::CloseRequested { api, .. } => { | ||
| // event.window().hide().unwrap(); | ||
| // api.prevent_close(); | ||
| // } | ||
| // _ => {} | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Since it's commented out, I assume this can be removed?
| @@ -9,6 +9,8 @@ export interface AppSettings { | |||
| oscReceivingPort: number; | |||
| enableDesktopNotifications: boolean; | |||
| enableXSOverlayNotifications: boolean; | |||
| exitInSystemTray: boolean; | |||
There was a problem hiding this comment.
nit: Can you refactor this to closeToSystemTray?
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct SystemTrayManager { | ||
| pub exit_in_tray: bool, |
There was a problem hiding this comment.
Same here I guess
| pub exit_in_tray: bool, | |
| pub close_to_tray: bool, |
|
As I've refactored a majority of Oyasumi's Rust core, this PR ended up requiring quite a bit more changes to fit into the new structure. I didn't really want to ask you to update your work to get it working again, so I've made a few changes for it myself so that it could be merged. You'll find the changes in 7776f99. Thanks a bunch for your contribution! |
Adds start and exit with system tray behaviours.
Both can be toggled on demand in general settings. There is some new entries in the i18n dictionary.
FYI : For now the splash screen is not hidden when start with system tray is enabled, only the main window is.